Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update read_health_facilities on python #347

Merged
merged 4 commits into from
Apr 25, 2024

Conversation

vss-2
Copy link
Contributor

@vss-2 vss-2 commented Apr 23, 2024

Implements the same feature present on R package and issued in #342 .

@rafapereirabr
Copy link
Member

Thanks @vss-2 . Your contribution is very much appreciated! The PR seems to failing all the tests, though. Could please have a look at this?

@vss-2
Copy link
Contributor Author

vss-2 commented Apr 24, 2024

Sure! The tests were failing because this first commit was sent before I send the other PR fixing the python tests pipeline.
I had to merge with main branch to update this issue.
There was also another error: I forgot to update expected dataset size on the python test.
I've fixed this, added the "banana" tests cases, also updated the error message to "year/date" on utils, following R version standard.

@vss-2
Copy link
Contributor Author

vss-2 commented Apr 24, 2024

Sorry, after tests didn't finishing well I noticed that adding a case where "verbose=banana" is acceptable in Python conditionals, yikes!
In R, the "banana" attribution to "showProgress" throws an exception, so tests aren't all that similar now hahahah.

@rafapereirabr
Copy link
Member

nice catch! thanks again @vss-2 !

@rafapereirabr rafapereirabr merged commit 139b38e into ipeaGIT:master Apr 25, 2024
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants